Skip to content

feat: configurable home link for the workspace rail - #178

Open
sercada wants to merge 1 commit into
openclaw:mainfrom
sercada:home-link-config
Open

feat: configurable home link for the workspace rail#178
sercada wants to merge 1 commit into
openclaw:mainfrom
sercada:home-link-config

Conversation

@sercada

@sercada sercada commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Deployments that run ClickClack inside a larger product (ours sits next to an internal operations app, behind Cloudflare Access) have a home button on the workspace rail that always leads to the ClickClack landing page and always reads cc. For those users the landing page is a dead end; the button should lead back to the product they came from.

Why This Change Was Made

Two optional server settings, CLICKCLACK_HOME_URL and CLICKCLACK_HOME_LABEL (home_url / home_label in the JSON config file), set the destination and the short label of that button. Both keep the built-in defaults (/, cc) when unset, so existing deployments do not change.

  • config: the URL must be an absolute http(s) URL or an absolute path on this deployment; javascript:, protocol-relative and relative values are rejected at serve validation. The label is trimmed and bounded (32 characters).
  • httpapi: new public GET /api/home-link returning {url, label} with defaults applied. It carries no user or workspace data.
  • web: lib/home-link.ts loads and re-validates the payload field by field (a malformed or failed response keeps the default); GuildRail renders the label and title; the desktop shell keeps its /app home when the default is in use.
  • docs/configuration.md: two new rows.
  • Embedded web assets rebuilt.

User Impact

No change unless the settings are configured. With them, the rail's home button shows the product name and opens the product.

Evidence

  • go build ./..., go vet, gofmt -l clean.
  • go test ./apps/api/internal/config/ ./apps/api/internal/httpapi/ -run 'TestNormalizeHomeLink|TestValidateServe|TestHomeLinkEndpoint|TestLoadDefaultsEnvAndFile': ok (defaults, absolute URL, absolute path, label-only, rejected javascript: / //host / relative / oversized label; endpoint defaults, configured, partial config).
  • node --test src/lib/home-link.test.ts: 4 pass (default fallback, accepted shapes, field-by-field rejection, failed fetch fallback).
  • pnpm typecheck (web), pnpm lint, pnpm fmt:check: clean.
  • Full pnpm test (web tests + web/sdk build + Go suite with the rebuilt dist): running at the time of opening; result appended below.

@sercada
feat: configurable home link for the workspace rail
29655c7 
Deployments that run ClickClack inside a larger product need the rail's
home button to lead back to that product instead of the ClickClack landing
page. CLICKCLACK_HOME_URL / CLICKCLACK_HOME_LABEL (home_url / home_label in
the config file) set the destination and the short label; both keep their
built-in defaults when unset.

The API validates the URL as an absolute http(s) URL or an absolute path
(never javascript: or protocol-relative), bounds the label, and serves the
pair from the public GET /api/home-link. The shell reads it once at startup
and renders it on the GuildRail home button; a failed or malformed response
keeps the default. Embedded web assets rebuilt.
@sercada
sercada requested a review from a team as a code owner August 27, 2026 23:37
@clawsweeper

clawsweeper Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@sercada

sercada commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Evidence, continued (full pnpm test from the repository root on this branch):

  • @clickclack/web node tests: pass (including the new home-link.test.ts).
  • pnpm build:web + pnpm build:sdk + Go suite against the rebuilt dist: all packages ok. One httpapi test (TestHTTPErrorPathsAndSPA, event-callback signature timing) failed in that first run while the machine was also running another Go build; it passes in isolation (-count=2) and the whole httpapi package passes -count=3 on this branch (38s) and on main (41s), so I read it as load-related flakiness rather than a regression. Happy to dig further if CI disagrees.

@clawsweeper

clawsweeper Bot commented Aug 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed August 27, 2026, 8:03 PM ET / August 28, 2026, 00:03 UTC.

ClawSweeper review

What this changes

The PR adds optional home-link URL and label settings, exposes them through the API, and renders them in the workspace rail.

Merge readiness

⛔ Blocked until real behavior proof from a real setup is added - 13 items remain

Keep this PR open: its focused implementation has three P2 correctness gaps and only test/CI evidence, not inspectable after-fix behavior proof.

Priority: P3
Reviewed head: 29655c74ca785fbea74179a99852a17c154839a2
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The focused feature has useful tests, but three reproducible behavior defects and no real-behavior proof keep it from merge readiness.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: The PR reports tests and CI but has no inspectable after-fix runtime evidence; after repair, post redacted browser or desktop navigation proof and update the PR body for re-review.
Patch quality 🦪 silver shellfish (2/6) Security review found an item that needs attention.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR reports tests and CI but has no inspectable after-fix runtime evidence; after repair, post redacted browser or desktop navigation proof and update the PR body for re-review.
Evidence reviewed 5 items Desktop default regression: A label-only configuration has URL / but fails the two-field default check, so the integrated desktop rail receives / rather than its established /app route.
Desktop path guard: The desktop navigation contract permits only safe /app routes with integrated chrome; a configured same-origin path such as /portal is prevented and is not opened externally.
Backslash URL bypass: The client accepts every value starting with /, but browser URL parsing resolves slash-plus-backslash to an external origin.
Findings 3 actionable findings [P2] Preserve desktop home when only the label is configured
[P2] Handle configured same-origin paths in the desktop app
[P2] Reject backslash paths before assigning them to href
Security Needs attention Home-link path validation permits external navigation: A slash-plus-backslash path passes the new checks but browser parsing treats it as an external host-relative navigation, defeating the documented same-deployment-path restriction.

Live Verification

Command: pnpm --filter @clickclack/web test

Result: PASS (completed)

✔ classifyArtifact detects text by extension, plain type, and the text/* fallback (0.187812ms)
✔ classifyArtifact returns unsupported for unknown extension and content type (0.141614ms)
✔ classifyArtifact lets the extension win over the content type in the cascade (0.078858ms)
✔ artifactLanguage maps known code extensions (0.101022ms)
✔ artifactLanguage falls back to content-type substrings when the extension is unknown (0.106249ms)
✔ artifactLanguage prefers the code extension over the content type (0.089645ms)
✔ artifactLanguage returns undefined when nothing matches (0.072719ms)
✔ artifactKindLabel returns a label for every kind and a default (0.151939ms)
✔ artifactPreviewLimit returns the per-family cap (0.113531ms)
✔ artifactPreviewLimit returns undefined for kinds without an inline preview (0.091888ms)
✔ accepts explicit light and dark modes on channel and thread embeds (1.011132ms)
✔ does not apply embed themes to normal ClickClack pages (0.154052ms)
✔ accepts only exact HTTP and HTTPS host origins on embed routes (0.165399ms)
✔ rejects host origins with paths, credentials, non-HTTP schemes, and normal app routes (0.262666ms)
✔ revokes the original host theme after leaving the embedded route (0.673794ms)
▶ home link
  ✔ keeps the ClickClack default when the payload is missing or malformed (1.331895ms)
  ✔ accepts an absolute http(s) URL or an absolute path with a short label (0.203536ms)
  ✔ refuses unsafe destinations and oversized labels field by field (0.145359ms)
  ✔ falls back to the default when the endpoint fails (0.210627ms)
✔ home link (2.846434ms)
✔ finds known handles at token boundaries (1.191703ms)
✔ does not highlight unknown handles, emails, or URL paths (0.169275ms)
✔ matches handles case-insensitively (0.109004ms)
✔ keeps non-recent ordinary workspace members available for mention highlighting (0.674796ms)
✔ isWorkspaceManager is true only for owner and moderator roles (0.713564ms)
✔ isWorkspaceManager treats missing roles as non-managers (0.129575ms)
✔ currentRole matches a workspace by id or by route_id (0.175214ms)
✔ currentRole returns undefined when the id is blank or unmatched (0.111688ms)
✔ currentRole surfaces an undefined role on a matched workspace (0.130346ms)
✔ recognizes the server resync close code (0.740365ms)
✔ uses an existing cursor without bootstrapping a fresh client (0.718381ms)
✔ captures a fresh tail as a pending post-open checkpoint (0.187812ms)
✔ authoritatively bootstraps a stored empty cursor (0.176276ms)
✔ does not return a checkpoint after the bootstrap attempt becomes stale (0.181173ms)
✔ propagates storage reads and bootstrap failures for reporting and retry (0.51178ms)
✔ processes events in arrival order and checkpoints only after success (1.083091ms)
✔ stops after a handler failure without checkpointing later events (0.24578ms)
✔ drains a large replay backlog after a blocked handler (2.018338ms)
✔ fails and clears a live backlog beyond the configured bound (0.28587ms)
✔ ignores queued work after the connection becomes stale (0.15287ms)
ℹ tests 59
ℹ suites 1
ℹ pass 59
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 411.453634

Assertions:

  • PASS expect_output: home link

How this fits together

The server validates deployment configuration and serves a small home-link payload to the ClickClack web shell. The shell loads it at startup and supplies the destination and label to the workspace navigation rail.

flowchart LR
A[Deployment settings] --> B[Server validation]
B --> C[Home-link API]
C --> D[Web shell startup]
D --> E[Desktop navigation guard]
D --> F[Workspace rail link]
Loading

Decision needed

Question Recommendation
Should home_url and home_label become a supported core deployment configuration surface after the correctness fixes? Sponsor the configuration: Accept the narrow deployment-navigation capability once the desktop, validation, and proof blockers are resolved.

Why: This PR adds permanent server configuration and a public endpoint; the mechanical fixes do not establish maintainer acceptance of that product contract.

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR reports tests and CI but has no inspectable after-fix runtime evidence; after repair, post redacted browser or desktop navigation proof and update the PR body for re-review.
  • Preserve desktop home when only the label is configured (P2) - isDefaultHomeLink requires both fields to be default. An integrated desktop that configures only CLICKCLACK_HOME_LABEL therefore gets / instead of its established /app route, where the desktop guard prevents navigation. Base this fallback on the URL alone and cover the label-only case.
  • Handle configured same-origin paths in the desktop app (P2) - The feature accepts paths such as /portal, but integrated desktop navigation allows only safe /app paths and the guard neither loads nor externally opens same-origin rejected paths. Route a configured non-app destination through the established safe external-navigation behavior. This case was present at the prior reviewed head and was caught in the continuity trace.
  • Reject backslash paths before assigning them to href (P2) - This accepts any value beginning with /, but browsers normalize slash-plus-backslash as a scheme-relative navigation and leave the deployment. Reject backslashes in both client and server validators, with matching regressions.
  • Resolve security concern: Home-link path validation permits external navigation - A slash-plus-backslash path passes the new checks but browser parsing treats it as an external host-relative navigation, defeating the documented same-deployment-path restriction.
  • Resolve merge risk (P1) - An integrated desktop deployment using only the new label setting loses its existing working /app home destination.
  • Resolve merge risk (P1) - Configured same-origin product paths cannot be opened from the integrated desktop shell.
  • Resolve merge risk (P1) - A slash-plus-backslash value can become an unintended external browser navigation despite the intended same-deployment-path restriction.
  • Complete next step (P2) - The three introduced defects have narrow mechanical repairs, although contributor runtime proof and maintainer product acceptance still remain before merge.
  • Improve patch quality - Repair the three desktop and URL-validation findings with focused coverage.
  • Improve patch quality - Post redacted after-fix browser or desktop navigation evidence.
  • Improve patch quality - Obtain maintainer confirmation that the new core configuration surface should ship.

Findings

  • [P2] Preserve desktop home when only the label is configured — apps/web/src/ChatApp.svelte:4184
  • [P2] Handle configured same-origin paths in the desktop app — apps/web/src/ChatApp.svelte:4184
  • [P2] Reject backslash paths before assigning them to href — apps/web/src/lib/home-link.ts:13-14
  • [low] Home-link path validation permits external navigation — apps/web/src/lib/home-link.ts:14
Agent review details

Security

Needs attention: The new deployment-configured link validator permits a browser URL-parsing bypass to an external host.

Review metrics

Metric Value Why it matters
Source and test delta production +169, tests +152, generated web assets +20 The feature is concentrated in server/web configuration and includes regenerated embedded frontend output.

Merge-risk options

Maintainer options:

  1. Repair desktop routing and validation (recommended)
    Preserve desktop /app for a default URL, safely open configured non-app destinations, and reject backslashes in both validators before merge.
  2. Pause the feature direction
    Defer this optional configuration if maintainers do not want to support another persistent deployment setting.
Copy recommended automerge instruction 
@clawsweeper automerge

Special instructions:
Preserve integrated desktop /app when only the label is configured; handle configured non-app home destinations through the established safe external-navigation path; reject backslashes in server and client URL validation; add focused regressions.

Technical review

Best possible solution:

Preserve /app for integrated desktops when the URL is still /, handle configured non-app destinations through the existing safe external-navigation path, and reject backslashes in both validators before adding redacted runtime proof.

Do we have a high-confidence way to reproduce the issue?

Yes. Current-head source deterministically routes an integrated desktop with a label-only setting to /, blocks configured same-origin non-app paths, and browser URL parsing resolves slash-plus-backslash to an external origin.

Is this the best way to solve the issue?

No. The feature needs the three focused desktop and URL-validation corrections, plus maintainer confirmation that this new core configuration contract is desired.

Full review comments:

  • [P2] Preserve desktop home when only the label is configured — apps/web/src/ChatApp.svelte:4184
    isDefaultHomeLink requires both fields to be default. An integrated desktop that configures only CLICKCLACK_HOME_LABEL therefore gets / instead of its established /app route, where the desktop guard prevents navigation. Base this fallback on the URL alone and cover the label-only case.
    Confidence: 0.99
  • [P2] Handle configured same-origin paths in the desktop app — apps/web/src/ChatApp.svelte:4184
    The feature accepts paths such as /portal, but integrated desktop navigation allows only safe /app paths and the guard neither loads nor externally opens same-origin rejected paths. Route a configured non-app destination through the established safe external-navigation behavior. This case was present at the prior reviewed head and was caught in the continuity trace.
    Confidence: 0.98
    Late finding: first raised on code an earlier review cycle already covered.
  • [P2] Reject backslash paths before assigning them to href — apps/web/src/lib/home-link.ts:13-14
    This accepts any value beginning with /, but browsers normalize slash-plus-backslash as a scheme-relative navigation and leave the deployment. Reject backslashes in both client and server validators, with matching regressions.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: found, but no applicable review policy affected this item.

Codex review notes: model internal, reasoning high; reviewed against 27bcc19b1cbe.

Labels

Label changes:

  • add P3: This is an optional deployment-navigation feature rather than an urgent existing-user outage.
  • add merge-risk: 🚨 compatibility: A label-only setting changes integrated desktop behavior from its established /app route.
  • add merge-risk: 🚨 security-boundary: New URL validation accepts a browser-parsing form that can leave the deployment origin.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR reports tests and CI but has no inspectable after-fix runtime evidence; after repair, post redacted browser or desktop navigation proof and update the PR body for re-review.

Label justifications:

  • P3: This is an optional deployment-navigation feature rather than an urgent existing-user outage.
  • merge-risk: 🚨 compatibility: A label-only setting changes integrated desktop behavior from its established /app route.
  • merge-risk: 🚨 security-boundary: New URL validation accepts a browser-parsing form that can leave the deployment origin.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR reports tests and CI but has no inspectable after-fix runtime evidence; after repair, post redacted browser or desktop navigation proof and update the PR body for re-review.

Evidence

Security concerns:

  • [low] Home-link path validation permits external navigation — apps/web/src/lib/home-link.ts:14
    A slash-plus-backslash path passes the new checks but browser parsing treats it as an external host-relative navigation, defeating the documented same-deployment-path restriction.
    Confidence: 0.98

Acceptance criteria:

  • [P1] go test ./apps/api/internal/config/ ./apps/api/internal/httpapi/ -run 'TestNormalizeHomeLink|TestValidateServe|TestHomeLinkEndpoint'.
  • [P1] pnpm --filter @clickclack/web test.
  • [P1] pnpm --filter @clickclack/desktop test.
  • [P1] pnpm typecheck.
  • [P1] pnpm lint.

What I checked:

Likely related people:

  • Peter Steinberger: Recent merged work spans the web shell, server configuration, and HTTP API surfaces involved here. (role: recent area contributor; confidence: medium; commits: 7f06e74f6455, 04402c823df1; files: apps/web/src/ChatApp.svelte, apps/api/internal/config/config.go, apps/api/internal/httpapi/server.go)
  • Shakker: The workspace rail’s earlier feature history includes this contributor’s commit. (role: earlier navigation contributor; confidence: medium; commits: 22a57e9c10be; files: apps/web/src/components/navigation/GuildRail.svelte)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)
  • reviewed 2026-08-27T23:40:53.183Z sha 29655c7 :: needs real behavior proof before merge. :: [P2] Preserve desktop home when only the label is configured | [P2] Reject backslash paths before assigning them to href

@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@sercada